Skip to content

Conversation

@mesilov
Copy link
Owner

@mesilov mesilov commented Nov 21, 2025

Implements full CRUD functionality for application settings storage:

Features:

  • ApplicationSetting entity with UUID v7 ID generation
  • Key-value storage tied to ApplicationInstallations
  • Unique constraint on (application_installation_id, key)
  • Repository with find/save/delete operations
  • CQRS Use Cases: Set (create/update), Get, Delete
  • Comprehensive unit and functional tests
  • Doctrine ORM XML mapping configuration

Architecture:

  • Follows DDD and CQRS patterns
  • Extends AggregateRoot for event support
  • Readonly classes for commands
  • Strict type validation in constructors
  • Proper exception handling

Database:

  • Table: application_setting
  • Fields: id, application_installation_id, key, value, created_at_utc, updated_at_utc
  • Indexes on application_installation_id
  • Unique constraint ensures no duplicate keys per installation

Tests:

  • Unit tests for entity validation and business logic
  • Functional tests for repository operations
  • Functional tests for all use case handlers
Q A
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #67
License MIT

Implements full CRUD functionality for application settings storage:

Features:
- ApplicationSetting entity with UUID v7 ID generation
- Key-value storage tied to ApplicationInstallations
- Unique constraint on (application_installation_id, key)
- Repository with find/save/delete operations
- CQRS Use Cases: Set (create/update), Get, Delete
- Comprehensive unit and functional tests
- Doctrine ORM XML mapping configuration

Architecture:
- Follows DDD and CQRS patterns
- Extends AggregateRoot for event support
- Readonly classes for commands
- Strict type validation in constructors
- Proper exception handling

Database:
- Table: application_setting
- Fields: id, application_installation_id, key, value, created_at_utc, updated_at_utc
- Indexes on application_installation_id
- Unique constraint ensures no duplicate keys per installation

Tests:
- Unit tests for entity validation and business logic
- Functional tests for repository operations
- Functional tests for all use case handlers
@gemini-code-assist
Copy link

Summary of Changes

Hello @mesilov, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request establishes a new ApplicationSettings bounded context to provide a structured and robust mechanism for storing and managing key-value settings specific to application installations. It introduces a new entity, a dedicated repository, and a set of CQRS-based use cases for setting, retrieving, and deleting these settings. This change significantly improves the application's ability to handle configuration data in a scalable and maintainable way, aligning with modern architectural best practices.

Highlights

  • New Bounded Context: ApplicationSettings: Introduces a dedicated bounded context for managing application-specific key-value settings, enhancing modularity and separation of concerns.
  • Full CRUD Functionality: Implements complete Create, Read, Update, and Delete (CRUD) operations for application settings, including entity definition, repository, and CQRS use cases.
  • Robust Data Model: Defines an ApplicationSetting entity with UUID v7 IDs, a unique constraint on (application_installation_id, key), and proper indexing for efficient storage and retrieval.
  • CQRS and DDD Principles: Adheres to Command Query Responsibility Segregation (CQRS) and Domain-Driven Design (DDD) patterns, utilizing readonly command classes, strict type validation, and extending AggregateRoot for event support.
  • Comprehensive Testing: Includes extensive unit tests for entity validation and business logic, as well as functional tests for repository operations and all CQRS use case handlers, ensuring reliability and correctness.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a well-architected ApplicationSettings bounded context, following DDD and CQRS principles. The separation of concerns into entities, repositories, and use case handlers is clean, and the feature is supported by a comprehensive set of unit and functional tests. My feedback focuses on a few areas to improve consistency and maintainability, such as centralizing validation logic, simplifying repository queries, and using more specific exception types for better error handling.

@mesilov mesilov marked this pull request as draft November 21, 2025 08:17
@mesilov mesilov added the enhancement New feature or request label Nov 21, 2025
claude and others added 24 commits November 21, 2025 08:21
Major improvements to ApplicationSettings bounded context:

1. Entity & Interface:
   - Add ApplicationSettingInterface with TODO to move to b24-php-sdk
   - Add b24UserId (nullable int) for personal settings
   - Add b24DepartmentId (nullable int) for departmental settings
   - Add scope validation (user/department mutually exclusive)
   - Add isGlobal(), isPersonal(), isDepartmental() methods
   - Update key validation: only lowercase latin letters and dots

2. Repository & Interface:
   - Add ApplicationSettingRepositoryInterface with TODO
   - Add findGlobalByKey() - find setting without user/dept scope
   - Add findPersonalByKey() - find by key + user ID
   - Add findDepartmentalByKey() - find by key + department ID
   - Add findByKey() - flexible search with optional filters
   - Add findAllGlobal() - all global settings
   - Add findAllPersonal() - all user settings
   - Add findAllDepartmental() - all department settings
   - Refactor findAll() - all settings regardless of scope

3. Database Schema:
   - Add b24_user_id column (nullable integer)
   - Add b24_department_id column (nullable integer)
   - Update unique constraint: (app_id, key, user_id, dept_id)
   - Add indexes for performance: user_id, dept_id, key

4. Use Cases (CQRS):
   - Update Set/Command with optional b24UserId, b24DepartmentId
   - Update Get/Command with scope parameters
   - Update Delete/Command with scope parameters
   - All handlers now use interface types
   - Update validation for new scope parameters

5. Services:
   - Add InstallSettings service for default settings creation
   - Support bulk creation of global settings on install
   - Skip existing settings to avoid duplicates
   - Provides getRecommendedDefaults() helper

6. CLI Command:
   - Add ApplicationSettingsListCommand (app:settings:list)
   - List all settings for portal
   - Filter by user ID (--user-id)
   - Filter by department ID (--department-id)
   - Show only global settings (--global-only)
   - Table output with key, value, scope, timestamps

7. Tests:
   - Update unit tests for new validation rules
   - Add tests for global/personal/departmental scopes
   - Add tests for scope validation
   - Test userId/departmentId validation

Key validation change: Only lowercase latin letters and dots allowed
Example valid keys: app.enabled, user.theme, feature.analytics

Settings hierarchy:
- Global: applies to entire installation
- Departmental: specific to department (overrides global)
- Personal: specific to user (overrides departmental & global)
…ssue #67)

Changes:
- Remove Get UseCase (UseCases now only for data modification)
- Add changedByBitrix24UserId field to track who modified settings
- Add isRequired field for frontend validation hints
- Create ApplicationSettingChangedEvent with old/new values and change tracking
- Move InstallSettings from root Services to ApplicationSettings namespace
- Update Set UseCase to support new fields
- Update all tests for new Entity constructor signature
- Add comprehensive tests for new fields and event emission

Entity changes:
- changedByBitrix24UserId: nullable int, tracks modifier
- isRequired: boolean, indicates required settings for frontend
- updateValue() now emits ApplicationSettingChangedEvent

Doctrine mapping updated with new fields:
- changed_by_b24_user_id (nullable)
- is_required (not null)
Major improvements to ApplicationSettings:

1. Enum ApplicationSettingStatus
   - Added enum with Active and Deleted states
   - Supports soft-delete pattern for data retention

2. Entity changes
   - Added status field (ApplicationSettingStatus)
   - Added markAsDeleted() method for soft-delete
   - Added isActive() method to check status
   - Updated Doctrine mapping with status field and index

3. Repository improvements
   - All find* methods now filter by status=Active
   - Added softDeleteByApplicationInstallationId() method
   - Soft-deleted records excluded from queries by default
   - Hard delete methods preserved for admin operations

4. UseCase Delete refactored
   - Changed from hard delete to soft-delete
   - Calls markAsDeleted() instead of repository->delete()
   - Preserves data for audit and recovery

5. New UseCase: OnApplicationDelete
   - Command and Handler for bulk soft-delete
   - Triggered when application is uninstalled
   - Soft-deletes all settings for installation
   - Maintains data integrity and history

6. Comprehensive tests
   - Unit tests for status and markAsDeleted()
   - Functional tests for soft-delete behavior
   - Tests verify deleted records persist in DB
   - Full test coverage for OnApplicationDelete

7. Documentation
   - Complete guide in docs/application-settings.md
   - Russian language documentation
   - Detailed examples and best practices
   - Architecture and concepts explained
   - CLI commands and API usage

Key benefits:
- Data retention for compliance and audit
- Ability to recover accidentally deleted settings
- Historical data analysis capabilities
- Safe uninstall with data preservation

Database schema:
- Added status column (enum: active/deleted)
- Added index on status for query performance
- Backward compatible with existing data
- Add findByApplicationInstallationIdAndKey() as alias for findGlobalByKey()
- Add findByApplicationInstallationId() as alias for findAll()
- Update interface with new methods
- Fixes test compatibility issues
- Add getEvents() method to retrieve pending events without clearing them
- Method is useful for testing event emission
- Fixes unit test errors for ApplicationSetting event tests
- All 29 unit tests now passing
- Remove event testing methods from ApplicationSettingTest
  - testUpdateValueEmitsEvent()
  - testUpdateValueDoesNotEmitEventWhenValueUnchanged()
- Revert AggregateRoot to original implementation (remove getEvents method)
- Events are emitted via emitEvents() but not directly tested in entity tests
- Follows pattern used in Bitrix24Account and other entities
- All 27 unit tests passing (55 assertions)
Changes:
- Fix PHPStan error in Set/Handler.php
  - Add intersection type annotation for AggregateRootEventsEmitterInterface
  - Import ApplicationSettingInterface explicitly
- Apply PHP-CS-Fixer formatting to 14 files
  - Fix doc comment periods
  - Fix constructor body formatting
  - Fix fluent interface formatting
- Move documentation from docs/ to src/ApplicationSettings/Docs/
- All 27 unit tests passing (55 assertions)
Major changes:
1. Remove getStatus() method
   - Removed from ApplicationSetting entity and interface
   - Updated all tests to use isActive() instead
   - Maintains encapsulation of status field

2. Refactor OnApplicationDelete to use domain methods
   - Removed softDeleteByApplicationInstallationId() from repository
   - Handler now fetches all settings and marks each as deleted
   - Better follows domain-driven design principles
   - Added deletedCount to logging

3. Remove backward compatibility methods
   - Removed findByApplicationInstallationIdAndKey() from repository
   - Updated all tests to use findGlobalByKey() directly
   - Cleaner repository interface

4. Add PHPDoc annotations
   - Added @return ApplicationSettingInterface[] to findByApplicationInstallationId()
   - Improves IDE support and type checking

5. Remove getRecommendedDefaults() static method
   - Removed from InstallSettings service
   - Updated documentation to reflect proper usage
   - Developers should define their own defaults

6. Refactor InstallSettings to use Set UseCase
   - Now uses Set\Handler instead of direct repository access
   - Follows CQRS pattern consistently
   - Removed direct entity instantiation
   - Simplified constructor (removed repository and flusher dependencies)

7. Add comprehensive unit tests for InstallSettings
   - Tests for default settings creation
   - Tests for logging behavior
   - Tests for global settings scope
   - Tests for empty settings array handling
   - 31 unit tests passing (66 assertions)

All tests passing:
- 31 unit tests with 66 assertions
- PHP-CS-Fixer formatting applied
- PHPStan errors only in unrelated test base classes
Changes:
1. Simplify Delete UseCase Command
   - Remove b24UserId and b24DepartmentId parameters
   - Delete now only works with global settings
   - Updated Handler to use findGlobalByKey()
   - Updated all tests

2. Fix repository method naming conflict
   - Rename findAll() to findAllForInstallation()
   - Avoids conflict with EntityRepository::findAll()
   - Updated all usages in UseCases and tests
   - Updated findByApplicationInstallationId() alias

3. Fix Doctrine XML mapping
   - Change enumType to enum-type (correct syntax for Doctrine ORM 3)
   - Fixes mapping validation errors

4. Add comprehensive contract tests for ApplicationSettingRepository
   - testCanFindPersonalSettingByKey - test personal scope
   - testCanFindDepartmentalSettingByKey - test departmental scope
   - testCanFindAllGlobalSettings - test global settings filtering
   - testCanFindAllPersonalSettings - test personal settings filtering
   - testCanFindAllDepartmentalSettings - test departmental settings filtering
   - testSoftDeletedSettingsAreNotReturnedByFindMethods - test soft-delete
   - testFindByKeySeparatesScopes - test scope separation
   - Total: 19 repository tests covering all scenarios

All unit tests passing: 31 tests, 66 assertions
Code style: PHP-CS-Fixer applied
Applied automated code modernization with Rector:
- Added #[\Override] attributes to overridden methods
- Renamed variables to match method return types
- Converted closures to arrow functions where appropriate
- Added return types to closures and arrow functions
- Simplified boolean comparisons
- Improved code readability

Applied PHP-CS-Fixer formatting for consistent code style.

All tests passing (31 tests, 66 assertions).
- Add PHPDoc annotations for mock types
- Remove unused variable in callback
- All unit tests passing (31 tests, 66 assertions)
- PHPStan errors reduced from 25 to 18
Removed methods:
- findAllGlobal(Uuid): array
- findAllPersonal(Uuid, int): array
- findAllDepartmental(Uuid, int): array
- deleteByApplicationInstallationId(Uuid): void
- findByApplicationInstallationId(Uuid): array

Changes:
- Updated ApplicationSettingRepositoryInterface - removed 5 methods
- Updated ApplicationSettingRepository - removed implementations
- Updated ApplicationSettingsListCommand - use findAllForInstallation with filtering
- Updated tests - removed related test methods
- Updated documentation - show filtering pattern

The API is now simpler with single findAllForInstallation() method.
Filtering by scope is done in application code using entity methods
(isGlobal(), isPersonal(), isDepartmental()).

All tests passing (174 tests, 288 assertions).
All linters clean (PHPStan, Rector, PHP-CS-Fixer).
…rvice

Major changes:
- Removed 6 redundant repository methods, kept only findAllForInstallation()
- Added documentation about uniqueness invariant (installation+key+user+department)
- Created InMemory repository implementation for fast unit testing
- Created SettingsFetcher service with cascading resolution logic (Personal > Departmental > Global)
- Added comprehensive tests for InMemory repository (9 tests)
- Added comprehensive tests for SettingsFetcher (10 tests)
- Applied Rector and PHP-CS-Fixer improvements

Files changed:
- Modified ApplicationSettingRepositoryInterface: removed findGlobalByKey, findPersonalByKey, findDepartmentalByKey
- Modified ApplicationSettingRepository: removed method implementations
- Modified UseCase handlers to use findAllForInstallation() with filtering
- Updated all functional tests to use new filtering approach
- Added documentation section about invariants and uniqueness constraints
- Created ApplicationSettingInMemoryRepository with helper methods
- Created ApplicationSettingInMemoryRepositoryTest with 9 comprehensive tests
- Created SettingsFetcher service with getSetting() and getSettingValue() methods
- Created SettingsFetcherTest with 10 tests covering all override scenarios

All tests pass (193 unit tests, PHPStan level 5, Rector, PHP-CS-Fixer)
…ion handling

Changes:
- Created SettingsItemNotFoundException for when setting not found
- Renamed getSetting() to getItem() in SettingsFetcher
- Removed nullable return type from getItem() - now throws exception instead
- Updated getSettingValue() to return non-nullable string
- Updated all tests to use getItem() instead of getSetting()
- Replaced null-check tests with exception expectation tests
- Applied Rector and PHP-CS-Fixer improvements

All tests pass (193 unit tests, PHPStan level 5, Rector, PHP-CS-Fixer)
Changes:
- Added findAllForInstallationByKey(Uuid, string) method to repository interface
- Implemented method in ApplicationSettingRepository (Doctrine) with query filtering
- Implemented method in ApplicationSettingInMemoryRepository
- Updated SettingsFetcher to use new method instead of fetching all settings
- Added 3 unit tests for InMemory repository implementation
- Added 3 functional tests for Doctrine repository implementation
- Applied Rector improvements (parameter naming consistency)

Optimization:
- SettingsFetcher now fetches only settings with matching key instead of all settings
- Reduces memory usage and improves performance when many settings exist
- Database query now filters by both installation ID and key

All tests pass (196 unit tests, PHPStan level 5, Rector, PHP-CS-Fixer)
…update table name

Major refactoring of the ApplicationSettings bounded context to improve naming consistency and separate concerns between creating and updating settings.

Changes:

1. Entity Renaming:
   - Renamed ApplicationSetting → ApplicationSettingsItem
   - Renamed ApplicationSettingInterface → ApplicationSettingsItemInterface
   - Renamed ApplicationSettingChangedEvent → ApplicationSettingsItemChangedEvent
   - Updated all references and imports across the codebase

2. Repository Renaming:
   - ApplicationSettingRepository → ApplicationSettingsItemRepository
   - ApplicationSettingRepositoryInterface → ApplicationSettingsItemRepositoryInterface
   - ApplicationSettingInMemoryRepository → ApplicationSettingsItemInMemoryRepository
   - Fixed PHPDoc and class references

3. Database Schema:
   - Updated XML mapping entity name to ApplicationSettingsItem
   - Changed table name from 'application_setting' to 'application_settings'
   - Renamed mapping file to match new entity name

4. Use Case Refactoring (Breaking Change):
   - Renamed UseCase\Set → UseCase\Create
   - Create use case now ONLY creates new settings
   - Create throws InvalidArgumentException if setting already exists
   - Added new UseCase\Update for updating existing settings
   - Update throws InvalidArgumentException if setting doesn't exist
   - Update automatically emits ApplicationSettingsItemChangedEvent

5. Services Updated:
   - InstallSettings now uses Create use case
   - SettingsFetcher updated with new interface names
   - All service references updated

6. Tests Updated:
   - Renamed all test files to match new entity names
   - Updated Create tests to verify exception on duplicate
   - Added comprehensive Update use case tests
   - Fixed all test references and assertions
   - All tests pass

7. Documentation:
   - Completely updated application-settings.md
   - Documented Create vs Update separation
   - Added examples for both use cases
   - Updated all code examples
   - Added exception handling documentation

All changes verified with:
- PHPStan: ✓ No errors
- PHP-CS-Fixer: ✓ No style issues
…on support

Улучшения SettingsFetcher для более удобной работы с настройками:

1. Добавлен LoggerInterface:
   - Логирование всех операций getItem и getValue
   - Debug логи для каждого найденного scope (personal/departmental/global)
   - Warning лог когда настройка не найдена
   - Error лог при ошибках десериализации

2. Переименован метод getSettingValue → getValue:
   - Более короткое и удобное имя
   - Все использования обновлены в документации

3. Добавлена поддержка автоматической десериализации:
   - Новый параметр class?: class-string для getValue()
   - Использует Symfony Serializer для десериализации JSON в объекты
   - Возвращает string если class не указан, object если указан
   - PHPStan типизация с @template для type-safety

Пример использования:
```php
// Строковое значение
$value = $fetcher->getValue($uuid, 'app.theme');

// Десериализация в объект
$config = $fetcher->getValue(
    uuid: $uuid,
    key: 'api.config',
    class: ApiConfig::class
);
```

4. Обновлены тесты:
   - Добавлены mocks для serializer и logger
   - Новые тесты для десериализации
   - Тест для проверки логирования ошибок
   - Тест что serializer не вызывается без class параметра

5. Обновлена документация:
   - Обновлен раздел "Сервис SettingsFetcher"
   - Добавлены примеры с десериализацией
   - Обновлен Пример 2 с показом обоих вариантов использования
   - Все ссылки на getSettingValue заменены на getValue

Все изменения проверены:
- PHPStan: ✓ No errors
- PHP-CS-Fixer: ✓ Fixed formatting
Rector автоматически применил следующие улучшения:

1. Переименование переменных для соответствия типам:
   - $applicationSetting → $applicationSettingsItem
   - $expectedObject → $testConfigDto
   - $e → $throwable (для exception variables)

2. Переименование параметров методов:
   - save($applicationSetting) → save($applicationSettingsItem)
   - delete($applicationSetting) → delete($applicationSettingsItem)

3. Улучшение arrow functions:
   - Преобразование closures в arrow functions где возможно
   - Добавление return types для arrow functions

4. Doctrine repository improvements:
   - ChildDoctrineRepositoryClassTypeRector applied

Применённые правила:
- RenameVariableToMatchNewTypeRector
- RenameParamToMatchTypeRector
- CatchExceptionNameMatchingTypeRector
- ClosureToArrowFunctionRector
- AddArrowFunctionReturnTypeRector
- ChildDoctrineRepositoryClassTypeRector

Все тесты пройдены:
- Unit tests: ✓ 199/199
- PHPStan: ✓ No errors
- PHP-CS-Fixer: ✓ No issues
- Add SettingsItemAlreadyExistsException for Create use case
- Update Create Handler to throw specific exception for duplicates
- Update Delete Handler to use SettingsItemNotFoundException
- Replace mock Serializer with real Symfony Serializer in tests
- Add comprehensive type deserialization tests (string, bool, int, float, DateTime)
- Add symfony/property-access dependency for ObjectNormalizer
- All unit tests passing (204/204)
Changed the ApplicationSettingsItem entity to generate its own UUID v7
in the constructor instead of receiving it as a constructor argument.
This improves encapsulation and follows DDD principles by making the
entity responsible for its own identity generation.

Changes:
- Modified ApplicationSettingsItem constructor to generate UUID internally
- Updated Create Handler to remove UUID generation from constructor call
- Updated all unit and functional tests to match new constructor signature
- All tests passing (204/204)
- PHPStan level 5: no errors
- PHP-CS-Fixer: no issues
Replaced the direct execution of `composer-license-checker` with a `docker-compose` command, aligning it with other linting and analysis workflows.

Signed-off-by: mesilov <[email protected]>
…t filtering logic

Introduced TODOs for missing methods in `b24-php-sdk` interface to improve future compatibility. Refactored `Bitrix24Account` handling to filter master accounts explicitly, ensuring accurate validation and exception handling. Removed redundant `#[\Override]` attributes and enhanced comments for clarity. Updated `CHANGELOG.md` to reflect related changes.

Signed-off-by: mesilov <[email protected]>
* This service is responsible for initializing default global settings
* when an application is installed on a Bitrix24 portal
*/
readonly class InstallSettings
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если это сервис, то это сервис и по неймингу. Переименовать в DefaultSettingsInstaller

Copy link
Collaborator

@camaxtly camaxtly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

23-11-2025

…n-memory implementation to test helpers, and focus implementation-specific tests on unique behavior.

Signed-off-by: mesilov <[email protected]>
…ion, and enhance Quick Start documentation

Signed-off-by: mesilov <[email protected]>
…sue-67-01Pg2AJibYNoBKmUVijCYFhU

# Conflicts:
#	src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php
#	src/ApplicationInstallations/UseCase/Install/Handler.php
#	src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php
#	src/ApplicationInstallations/UseCase/Uninstall/Handler.php
…n `findMasterAccountByMemberId` method

Signed-off-by: mesilov <[email protected]>
…or explicit persistence, adjust variable naming in commands, and simplify unique constraint validation

Signed-off-by: mesilov <[email protected]>
…e redundant TODO comments and PHPStan ignore directives

Signed-off-by: mesilov <[email protected]>
@mesilov mesilov marked this pull request as ready for review November 23, 2025 19:21
@mesilov mesilov requested a review from camaxtly November 23, 2025 19:22
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


<field name="isRequired" type="boolean" column="is_required" nullable="false"/>

<field name="status" enum-type="string" column="status" nullable="false"/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Map status enum using enum class, not string literal

The XML mapping declares status with enum-type="string", but ApplicationSettingsItem::$status is a typed ApplicationSettingStatus enum (see src/ApplicationSettings/Entity/ApplicationSettingsItem.php:30‑39). Doctrine expects enum-type to point at the enum FQCN; using the literal string will hydrate database values as plain strings and will either raise a mapping/type error or fail assigning to the enum-typed property whenever settings are loaded from the database. Any read of application settings will break until the mapping references ApplicationSettingStatus (or a custom type) instead of string.

Useful? React with 👍 / 👎.

Comment on lines +30 to +32
<unique-constraints>
<unique-constraint columns="application_installation_id,key,b24_user_id,b24_department_id" name="unique_app_setting_scope"/>
</unique-constraints>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Soft-deleted settings still block re-create via unique key

The unique constraint on (application_installation_id,key,b24_user_id,b24_department_id) applies regardless of status, but the Delete handler only soft-deletes by setting status to Deleted (src/ApplicationSettings/UseCase/Delete/Handler.php:53‑55). After a setting is deleted, the row remains and the unique constraint still matches, so a subsequent Create for the same key/scope will hit a database unique-constraint violation even though code only checks active records. Include status in the unique key or hard-delete soft-deleted rows to allow recreating settings after deletion.

Useful? React with 👍 / 👎.

…nheritance, simplify constructor, and replace getEntityManager calls with entityManager property access

Signed-off-by: mesilov <[email protected]>
…y and refactor related code, tests, and documentation.

Signed-off-by: mesilov <[email protected]>
…emantics; update references, documentation, tests, and log prefixes.

Signed-off-by: mesilov <[email protected]>
@mesilov mesilov merged commit 7aec333 into dev Nov 24, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Добавить поддержку хранения настроек приложения

4 participants